fix: always send both GraphQL-Features flags when assigning Copilot to an issue#39719
Conversation
…header Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…header per GitHub docs Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Ensures Copilot issue assignment mutations always include the full set of required GraphQL-Features flags so sessions reliably start even when no explicit model is provided.
Changes:
- Always send
issues_copilot_assignment_api_support,coding_agent_model_selectioninGraphQL-Featuresfor the primary assignment mutation. - Apply the same
GraphQL-Featuresflags to theaddAssigneesToAssignablefallback path. - Update unit test expectations to match the new always-send-both-flags behavior.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/assign_agent_helpers.cjs | Makes GraphQL-Features include both required flags for the primary assignment call and the fallback mutation path. |
| actions/setup/js/assign_agent_helpers.test.cjs | Updates assertions to expect both feature flags regardless of whether model is provided. |
| .github/workflows/release.lock.yml | Updates safe-outputs validation schema for update_release.body (adds minLength). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 3
| "required": true, | ||
| "type": "string", | ||
| "sanitize": true, | ||
| "maxLength": 65000 | ||
| "maxLength": 65000, | ||
| "minLength": 20 |
| const fallbackResp = await githubClient.graphql(fallbackMutation, { | ||
| assignableId, | ||
| assigneeIds: [agentId], | ||
| headers: { | ||
| "GraphQL-Features": "issues_copilot_assignment_api_support", | ||
| "GraphQL-Features": "issues_copilot_assignment_api_support,coding_agent_model_selection", | ||
| }, |
| expect(variables.headers["GraphQL-Features"]).toBe("issues_copilot_assignment_api_support"); | ||
| expect(variables.headers["GraphQL-Features"]).toBe("issues_copilot_assignment_api_support,coding_agent_model_selection"); | ||
| }); | ||
|
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #39719 does not have the 'implementation' label and has 0 new lines of code in business logic directories (≤100 threshold). |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Test Quality Sentinel completed test quality analysis. |
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnose and /zoom-out — leaving comments, no blocking issues.
📋 Key Themes & Highlights
Key Themes
- Missing fallback regression test: The
addAssigneesToAssignablefallback path is now fixed, but the fallback code branch has no test coverage at all. The PR correctly updated the primary-path test, but the same diligence wasn't applied to the fallback. Given that this was a silent failure (API returns 200, session never starts), a regression test here is especially valuable. - Unrelated lock file change:
release.lock.ymlgains aminLength: 20constraint that isn't mentioned in the PR description — worth verifying it's intentional.
Positive Highlights
- ✅ Root cause properly fixed: Both code paths updated, not just the one that appeared in the ternary. The fallback path's bug was actually more severe (always broken, regardless of
model). - ✅ Tight, minimal diff: 3 production lines, 3 test lines — exactly the right scope for a correctness fix.
- ✅ Test updated to reflect new invariant: The existing no-model test now correctly guards the both-flags requirement rather than encoding the broken behavior.
- ✅ Clear comment on the constant:
// Both feature flags are required per GitHub Copilot documentationgives future readers the why without requiring them to look up the docs.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer
Comments that could not be inline-anchored
actions/setup/js/assign_agent_helpers.test.cjs:493
[/diagnose] The fallback path (addAssigneesToAssignable) was fixed to include both flags, but has zero test coverage — there's no test that drives the fallback branch at all, let alone one that guards the header value. Without a regression test, this fix could silently regress in the same way as the original bug.
<details>
<summary>💡 Suggested regression test for the fallback path</summary>
Add a new it block inside the describe("assignAgentToIssue") suite, just before the clos…
.github/workflows/release.lock.yml:663
[/zoom-out] This minLength: 20 addition appears unrelated to the GraphQL-Features fix described in the PR. Lock files are generated artifacts — if this change was intentional (e.g. pulled from an upstream source bundle), it should be called out in the PR description so reviewers can verify it wasn't accidentally included.
<details>
<summary>💡 What to check</summary>
- If unintentional: rebase from
mainand re-runmake recompileto see if the diff disappears. - If *intentional…
There was a problem hiding this comment.
Request to address outstanding comments before merge
The core fix is correct: always emitting both issues_copilot_assignment_api_support,coding_agent_model_selection flags on both the primary and fallback paths addresses the root cause described in the PR.
However, three concerns from the existing review thread should be resolved first:
Blocking concerns
-
Fallback fix is untested (line 486) — the
addAssigneesToAssignablepath was silently broken before this PR and the new behavior has no corresponding test. If it regresses nobody will know. -
Duplicate string constant (line 487) —
const graphqlFeaturesis declared inside thetryblock; JSconstscoping makes it unreachable in thecatchblock, so the string must be duplicated. Hoisting the constant above thetry/catchwould eliminate the drift risk. -
Unrelated
release.lock.ymlchange (line 665) — addingminLength: 20to the release-body schema is out of scope for this PR. Confirm intentional or revert.
🔎 Code quality review by PR Code Quality Reviewer
🧪 Test Quality Sentinel Report✅ Test Quality Score: 85/100 — Excellent
📊 Metrics & Test Classification (2 tests analyzed)
Go: 0 ( Scoring breakdown:
Notes:
Verdict
|
The GitHub Copilot docs require both
issues_copilot_assignment_api_supportandcoding_agent_model_selectionin theGraphQL-Featuresheader for every assignment mutation. The second flag was only sent when an explicitmodelwas provided — without it, the API accepts the call and returns success but never starts a Copilot session.Changes
assign_agent_helpers.cjs—graphqlFeaturesis now unconditionally"issues_copilot_assignment_api_support,coding_agent_model_selection"instead of conditional onmodelassign_agent_helpers.cjs(fallback) — same fix for theaddAssigneesToAssignablefallback path, which also only sent the first flagassign_agent_helpers.test.cjs— updated assertion that was explicitly testing for the single-flag behavior when no model is provided